[Symfony] Set few default common service names for Symfony App Analyzer#1687
Conversation
stloyd
force-pushed
the
symfony-app-analyzed
branch
from
166d2a7 to
325a6d7
Compare
| /** | ||
| * @param ParameterProvider $parameterProvider | ||
| * @param SymfonyKernelParameterGuard $symfonyKernelParameterGuard | ||
| * @param ContainerFactory $containerFactory |
There was a problem hiding this comment.
There should be no duplicated for PHP code info.
The sniff is disabled for this, because it was doing to much
| $this->parameterProvider = $parameterProvider; | ||
| $this->symfonyKernelParameterGuard = $symfonyKernelParameterGuard; | ||
| $this->containerFactory = $containerFactory; | ||
| $this->commonNamesToTypes = array_merge($this->commonNamesToTypes, $commonNamesToTypes); |
There was a problem hiding this comment.
What is this needed for? Default names should not depend on custom user input, but on default Symfony naming.
The point is 0-configuration. So services are loaded from the App container itself and we don't bother user to basically copy-paste it to rector.yaml config :)
There was a problem hiding this comment.
Problem is for example that by default container says that: doctrine.orm.entity_manager is instance of Doctrine\Common\Persistence\ObjectManager, which in fact is not really true, cause it's instance of Doctrine\ORM\EntityManagerInterface (which is also used for autowire functionality).
This is just one example, if there is more edge cases (in big legacy like we have, we can occur them most likely), user is not able to have automatic fix for such cases and he/she will be forced to fix it manually.
With this you can have most common known cases for replacement, but also user is able to not wait for new release of Rector if he/she finds another edge case.
There was a problem hiding this comment.
This is just one example, if there is more edge cases (in big legacy like we have, we can occur them most likely), user is not able to have automatic fix for such cases and he/she will be forced to fix it manually.
I see. It makes sense now :) 👍
I miss this description on PR. Could you add it next time, so I can read the code with context of your intention? It would speed up the review
Allow to set custom ones via configuration
stloyd
force-pushed
the
symfony-app-analyzed
branch
from
325a6d7 to
6d23c75
Compare
rectorphp/rector-src@ca696bc [DowngradePhp74] Do not remove non-null default value on nullable on DowngradeTypedPropertyRector (#1687)
2 participants
Allow to set custom ones via configuration: